[#763] Fix plot count +1: deduplicate and add unique constraint#766
[#763] Fix plot count +1: deduplicate and add unique constraint#766realproject7 merged 3 commits intomainfrom
Conversation
- Migration 00030: deletes duplicate (storyline_id, plot_index) rows keeping the earliest entry, adds unique constraint to prevent recurrence, and re-reconciles all plot counts - Updates all plot upsert calls (indexer, storyline indexer, backfill) to use onConflict: "storyline_id,plot_index" with ignoreDuplicates instead of tx_hash,log_index — prevents duplicate genesis plots - Updates reconcile.ts comment to document the constraint dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE
Well-structured fix across migration + indexer routes:
- Migration: Correct order — dedupe (keep MIN id), add unique constraint, then reconcile.
IS DISTINCT FROMin the reconcile UPDATE is a nice touch to skip no-op rows. - Upsert changes: Switching conflict key from
tx_hash,log_indextostoryline_id,plot_indexwithignoreDuplicates: truedirectly addresses the root cause (same plot indexed via different tx combos). Safe because plot data is immutable per index on-chain. - Comment in reconcile.ts documents the constraint dependency.
No concerns.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The unique-constraint direction makes sense for fixing duplicate plot counts, but the current conflict handling introduces a metadata regression for genesis plots.
Findings
- [high] Genesis plot titles can now be lost permanently. The storyline/backfill genesis insert paths still write the
plotsrow without atitle, but this PR changes those inserts toonConflict: "storyline_id,plot_index", ignoreDuplicates: true. That means if the genesis row is created first by the storyline path, the later plot-indexer write that does includetitleis discarded, leaving the row at the default empty string forever.- File:
src/app/api/index/storyline/route.ts:162 - File:
src/app/api/cron/backfill/route.ts:292 - Suggestion: Either include
titlein the genesis plot rows written by the storyline/backfill paths, or keep the new conflict key but allow merge updates instead ofignoreDuplicates: trueso the later richer row can populate missing fields.
- File:
Decision
Requesting changes because the current PR fixes the count issue by turning duplicate genesis writes into no-ops, but that also drops legitimate metadata from the surviving row.
Genesis insert paths (storyline indexer, backfill genesis) keep ignoreDuplicates: true since they insert bare rows without title. Plot indexer and backfill plot paths use regular upsert so they can update rows with full data (title, etc.) if a genesis row exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The new conflict-handling split fixes the future genesis-write race, but the migration still drops metadata from already duplicated genesis plots.
Findings
- [high] The migration deletes duplicate
(storyline_id, plot_index)rows by keepingMIN(id), which will preserve the earlier bare genesis row and delete the later duplicate row that carries the genesistitle. The follow-up upsert fix only helps future replays; it does not repair the duplicated rows that already exist and are being deduplicated here.- File:
supabase/migrations/00030_dedupe_plots_unique_constraint.sql:7 - File:
src/app/api/index/storyline/route.ts:162 - File:
src/app/api/index/plot/route.ts:117 - Suggestion: Before deleting duplicates, merge non-empty metadata from the duplicate set into the survivor, or choose the survivor from the richer row set instead of blindly keeping the lowest
id.
- File:
Decision
Requesting changes because the PR still turns an existing data-integrity bug into a persistent metadata-loss bug during migration for already duplicated genesis plots.
Migration now uses DISTINCT ON with ordering that prefers rows with non-empty title over bare genesis rows, then latest id as tiebreaker. This preserves title data from the plot indexer instead of keeping the earlier bare genesis row. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up migration change resolves the remaining data-loss issue by deduplicating (storyline_id, plot_index) rows in a way that prefers richer genesis rows before the unique constraint is added.
Findings
- None.
Decision
Approving because the PR now both prevents future duplicate plot rows and preserves the better existing row during migration, which addresses the plot-count bug without the metadata regression from the earlier revision.
2 participants
Summary
(storyline_id, plot_index)duplicates (keeps earliest), addsUNIQUE(storyline_id, plot_index)constraint, re-reconciles all plot countsonConflictfromtx_hash,log_indextostoryline_id,plot_indexwithignoreDuplicates: true— prevents genesis plot being double-inserted by both storyline and plot indexersFixes #763
Test plan
npm run buildpasses🤖 Generated with Claude Code